Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instructions and processor for stake deactivate and withdraw #2

Conversation

pgarg66
Copy link

@pgarg66 pgarg66 commented Jun 21, 2019

Problem

Missing instruction for deactivating stake

Summary of Changes

Added code and tests

solana-labs#4357

@pgarg66
Copy link
Author

pgarg66 commented Jun 21, 2019

@rob-solana

///
/// Expects 1 Accounts:
/// 0 - Delegate StakeAccount
Deactivate,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one of these at a time

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/// Deactivates the stake in the account
///
/// Expects 1 Accounts:
/// 0 - Delegate StakeAccount
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need 2: the delegate and syscall::current

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if rest.len() != 0 {
Err(InstructionError::InvalidInstructionData)?;
}
me.deactivate_stake(&syscall::current::from_keyed_account(&me)?)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest.len() should be exactly 1 and current comes from &rest[0]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

};
if lamports > self.account.lamports.saturating_sub(staked) {
return Err(InstructionError::InsufficientFunds);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also lower stake.stake to whatever remains in the account

Copy link
Author

@pgarg66 pgarg66 Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -284,6 +277,41 @@ mod tests {
),
Err(InstructionError::InvalidAccountData),
);

// gets the deserialization checks in deactivate
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure comments are correct

@rob-solana rob-solana force-pushed the move-redemption-to-rewards-pools branch from dcfc8da to 38ce28a Compare June 22, 2019 02:12
@rob-solana
Copy link
Owner

these can land on master now

@rob-solana rob-solana closed this Jun 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants